Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Move Braintree initializer into to_prepare block #257

Closed
wants to merge 1 commit into from
Closed

Move Braintree initializer into to_prepare block #257

wants to merge 1 commit into from

Conversation

pelargir
Copy link
Contributor

This fixes a probably load order error that can occur when Rails controller caching is disabled in development mode.

This error can be reproduced by running Solidus, Alchemy, and this extension together in development mode with perform_caching = false in the development.rb environment file. Attempting to either (1) start checkout or (2) view the details of a previously placed order will result in this error.

Turning on caching resolves the problem. Turning it off causes the problem to occur again.

I'm not 100% sure what's going on, but it seems to involve the braintree.rb initializer. If I comment out the following lines, the error goes away:

Spree::CheckoutController.helper :braintree_checkout
Spree::OrdersController.helper :braintree_checkout

Placing the entire initializer into a config.to_prepare block also resolves the problem, and seems like the right thing to do given that most other Solidus extensions use to_prepend blocks when adding helpers into controllers.

Screen Shot 2020-07-21 at 11 12 25 AM

This fixes a loading error that can occur when Rails controller caching
is disabled in development mode.
@seand7565
Copy link
Contributor

Hi @pelargir ! My suspicion with this issue is that it had to do with #252 - that PR was merged, and I think it fixed this issue as well. Can you verify this if you have the time? 🙏

@kennyadsl
Copy link
Member

Yep, should be fixed now. Thanks @pelargir for the detailed explanation and proposed fix, sorry for the long wait!

@kennyadsl kennyadsl closed this Oct 21, 2020
@pelargir
Copy link
Contributor Author

pelargir commented Oct 31, 2020

@seand7565 @kennyadsl yes it's fixed now. Again, it's frustrating to have spent time on a PR only to have it rendered useless by later work (see #258 for another example). How can I get PRs noticed more quickly? I try not to be annoying by posting stuff like "has anyone seen this yet?" but I'm not sure what else to do.

I'm sure this isn't done on purpose. I know it can be challenging managing an open source project, especially when it's not your primary job. However, it's disheartening to be submitting my first few PRs to a project and have them ignored for so long.

I'm guessing I'm not the only one in this boat. I know you want more people contributing to the project but when small, entry-level contributions like this get pushed aside it makes the submitters feel like not trying again.

@seand7565
Copy link
Contributor

Hey @pelargir - yeah, that's definitely frustrating! I completely understand where you're coming from.

I can't speak for anyone else, but I do know firsthand how difficult it is to organize efforts around Solidus. Solidus itself is already a massive time sink when it comes to maintaining, but then of course all of the extensions also need to be maintained, and sometimes extensions like this one slip through the net. I'm picking it back up - which is why there's a lot of activity here recently - but part of that is having to handle old PRs and issues, and lots of times there are duplicates.

But we definitely don't want effort put into PRs like the ones you made to go to waste, or discourage people from contributing. Honestly, I think it might be a good idea to have a "status check" channel or point of contact for things like this, so that we can avoid situations where PRs sit unattended for months or years. Maybe even a slack bot that posts stale PRs and prompts for updates?

I think there are a few things we can think about as possible solutions for this issue. If you have any specific ideas, please let me know! But in the meantime, if you have any other PRs that have gone stale, please don't hesitate to DM me on Slack about them, or tag me in the issue itself. I'll make sure they get routed to the right people.

Thanks for all the effort!

@pelargir
Copy link
Contributor Author

pelargir commented Nov 4, 2020

@seand7565 thanks for the response. That's very helpful. I like the idea to have a dedicated PR status channel. I'll be sure to reach out on Slack in the future if it seems like a PR isn't being seen.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants